Skip to content

refactor: clean up invalid log tags (#426) - #684

Open
akkicodes-dev wants to merge 1 commit into
scribe-org:mainfrom
akkicodes-dev:refactor/426-log-tags-cleanup
Open

refactor: clean up invalid log tags (#426)#684
akkicodes-dev wants to merge 1 commit into
scribe-org:mainfrom
akkicodes-dev:refactor/426-log-tags-cleanup

Conversation

@akkicodes-dev

Copy link
Copy Markdown

Summary

Cleans up invalid, typo'd, and generic log tags identified as part of #426.

Changes

  • KeyboardBase.kt: Replaced invalid unicode symbol tag "≠" with proper TAG constant ("KeyboardBase")
  • KeyboardView.kt: Fixed typo'd tag "Souncheck"TAG constant ("SoundCheck")
  • LanguageSettingsScreen.kt: Replaced generic, clashing "Navigation" tag (used in 3 places) with specific TAG constant ("LanguageSettingsScreen")

Testing

  • ./gradlew lintKotlin detekt — passed, no issues
  • ./gradlew test — all tests passed
  • Manually tested on a physical device: keyboard typing, sound feedback, and Language Settings navigation all work as expected with no regressions

Note

Parts of this PR were AI-assisted (Antigravity); all changes were manually reviewed line-by-line, verified via local build/lint/test, and tested on a physical device before submission.

Part of #426.

@andrewtavis

Copy link
Copy Markdown
Member

@prince-0408, would you be able to take a look? Thanks so much for the PR, @akkicodes-dev!

@prince-0408

Copy link
Copy Markdown
Collaborator

@prince-0408, would you be able to take a look? Thanks so much for the PR, @akkicodes-dev!

@andrewtavis Sure! on it!

@prince-0408

Copy link
Copy Markdown
Collaborator

LGTM!

Reviewed the changes across all 3 files:

  • Replaced invalid Unicode tag "≠" in KeyboardBase.kt with TAG = "KeyboardBase".
  • Replaced generic "Navigation" log tags in LanguageSettingsScreen.kt with TAG = "LanguageSettingsScreen".
  • Fixed typo'd "Souncheck" tag in KeyboardView.kt with a TAG constant.

Thanks @akkicodes-dev!

@andrewtavis

Copy link
Copy Markdown
Member

Discussing this with @angrezichatterbox right now, we're wondering what the use would be for keeping these logs at all. It might be nice to have an audit of the current logs that we have and whether they're useful for the development process :)

@akkicodes-dev

Copy link
Copy Markdown
Author

Thanks @prince-0408! And that's a great point @andrewtavis — happy to help with a logs audit if that would be useful. For now, should I proceed with this PR as-is since it fixes the immediate issues (invalid/typo'd/clashing tags), or would you prefer to hold off until the broader audit discussion concludes?

@angrezichatterbox

Copy link
Copy Markdown
Member

Hey @akkicodes-dev
If you are interested could u do a small audit of the logs. Wheather there are places were Logs are not needed are were added for personal debugging. The logs could also be checked for wheather its causing the logcat to be noisy with all sorts of information which is not neccesary. This would be very helpful to get a good idea of the repository as well.

Thanks : )

@akkicodes-dev

Copy link
Copy Markdown
Author

Thanks @angrezichatterbox! Yes, I'd be happy to take this on

To scope it correctly, a couple of quick questions:

  1. Should I cover the entire codebase (both keyboardDebug and conjugateDebug variants), or focus on one first?
  2. For logs that look unnecessary or debug-only, would you prefer I compile a report/list here for discussion first, or open a separate PR directly with the removals?

I'll start going through it and share my findings soon!

@akkicodes-dev

Copy link
Copy Markdown
Author

Completed the log audit! Went through all Log.* calls in the codebase (76 total across app/src/main and app/src/keyboards).

Summary:

  • USEFUL: 69 (tied to real exception handling — DB errors, network failures, parsing errors, etc. — these should stay)
  • LEFTOVER: 6 (look like debug traces left over from development)
  • NOISY: 1 (fires on every keystroke)

Here are the 7 candidates for removal:

File Line Log Reason
KeyHandler.kt 332 "New conjugate_index: $currentValue" Debug trace
ContractDataLoader.kt 30 "Attempting to load contract: $contractName" Debug trace
KeyboardBase.kt 225 "The current state is not conjugate view" Debug trace
LanguageSettingsScreen.kt 392 "onDefaultLayoutSelect clicked" Click event trace
LanguageSettingsScreen.kt 403 "onCurrencySelect clicked" Click event trace
LanguageSettingsScreen.kt 475 "onTranslationLanguageSelect clicked" Click event trace
KeyboardView.kt 690 "soundIfNeeded: $setSound" Fires on every keystroke — noisiest one

Want me to open a follow-up PR removing these 7, or would you like to review first?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants